Skip to content

Conversation

@isVoid
Copy link

@isVoid isVoid commented Jan 13, 2026

Description

closes #156

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Jan 13, 2026

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@leofang leofang requested review from leofang and mdboom January 13, 2026 19:00
@leofang leofang added P0 High priority - Must do! feature New feature or request cuda.bindings Everything related to the cuda.bindings module labels Jan 13, 2026
@leofang leofang added this to the cuda-python 13-next, 12-next milestone Jan 13, 2026
@leofang
Copy link
Member

leofang commented Jan 13, 2026

FYI @rparolin all of these are generated code except for the tests. There is an internal MR targeting the codegen for this.

Comment on lines 157 to 159
nvcc = shutil.which("nvcc")
if nvcc is None:
pytest.skip("nvcc not found on PATH")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a #TODO comment to use pathfinder to resolve this in the future instead? I imagine this wouldn't cover using the nvidia-cuda-nvcc wheel package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a TODO is fine, though it's unlikely actionable in the near future. We do not have access to host compilers in the CI (at the test stage) so NVCC is useless.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively I can also embed an object file in this test script, just like the tileIR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way is fine as long as we know how to reproduce the embedded bytes

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just looked up the size of an object file - with the small kernel here it's over 1.2M. Unlike tile IR this will clutter the test file. I'm going to leave this as a TODO for now.

if err != nvrtc.nvrtcResult.NVRTC_SUCCESS:
raise RuntimeError(repr(err))

err, program_handle = nvrtc.nvrtcCreateProgram(CODE.encode(), b"", 0, [], [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker for this PR, but I think we have some helper functions for NVRTC that can be used in cuda-bindings tests, so that we do not have to reimplement them in this file.

Copy link
Author

@isVoid isVoid Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I saw this pattern showed up in test_nvjitlink as well. Perhaps a shared fixture that returns an nvrtc artifact.

@leofang leofang added the to-be-backported Trigger the bot to raise a backport PR upon merge label Jan 15, 2026
@leofang
Copy link
Member

leofang commented Jan 15, 2026

/ok to test 8e75d8a

@github-actions
Copy link

@leofang
Copy link
Member

leofang commented Jan 15, 2026

@isVoid

  • The Windows builds fail with this error:
  cuda/bindings\nvfatbin.cpp(3718): error C2440: '=': cannot convert from 'const char *' to 'char *'

I suspect it's coming from get_error_string and we might need to cast away the constness, could you take a look?

@leofang
Copy link
Member

leofang commented Jan 15, 2026

  • For test failures with wheels, we need to update this dependency:
    "cuda-toolkit[nvrtc,nvjitlink,nvvm]==13.*",
  • Please add docs:
    • Take a look at cuda_bindings/docs/source/api.rst to see how we add API references
    • Need to update the release notes for both 13.1.X (we'll decide later if we'd do 13.1.2 or 13.2.0) and 12.9.6

@isVoid
Copy link
Author

isVoid commented Jan 15, 2026

/ok to test 965c53b

@leofang
Copy link
Member

leofang commented Jan 15, 2026

E   cuda.bindings._internal.utils.FunctionNotFoundError: function nvFatbinAddTileIR is not found

Need to skip the tile IR test for nvFatbin < 13.1

@isVoid
Copy link
Author

isVoid commented Jan 15, 2026

stderr = b'gcc: No such file or directory\nnvcc fatal : Failed to preprocess host compiler properties.\n'

This shows in some CI environment there's nvcc installed but it's not properly setup due to missing host compiler.

@isVoid
Copy link
Author

isVoid commented Jan 15, 2026

/ok to test 4baeecd

@isVoid
Copy link
Author

isVoid commented Jan 16, 2026

/ok to test 4c2f3f3

@leofang
Copy link
Member

leofang commented Jan 16, 2026

stderr = b'gcc: No such file or directory\nnvcc fatal : Failed to preprocess host compiler properties.\n'

This shows in some CI environment there's nvcc installed but it's not properly setup due to missing host compiler.

Yup! See #1467 (comment).

[project.optional-dependencies]
all = [
"cuda-toolkit[nvrtc,nvjitlink,nvvm]==13.*",
"cuda-toolkit[nvrtc,nvjitlink,nvvm,nvfatbin]==13.*",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kkraus14 this minds me a discussion we had a few months ago -- One day installing cuda-binding[all] would bring in the whole CTK 😂

@leofang
Copy link
Member

leofang commented Jan 16, 2026

LGTM! Thanks, @isVoid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module feature New feature or request P0 High priority - Must do! to-be-backported Trigger the bot to raise a backport PR upon merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add nvfatbin bindings

4 participants